Document guarantees of poisoning #144185
Merged
Document guarantees of poisoning #144185
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
This mostly documents the current behavior of `Mutex` and `RwLock` as imperfect. It's unlikely that the situation improves significantly in the future, and even if it does, the rules will probably be more complicated than "poisoning is completely reliable", so this is a conservative guarantee. We also explicitly specify that `OnceLock` never poisons, even though it has an API similar to mutexes.
purplesyringa
force-pushed
the
poisoning-wording
branch
from
029caf5 to
5b2c61e
Compare
This comment was marked as resolved.
This comment was marked as resolved.
rustbot
added
A-concurrency
rustbot
removed
the
S-waiting-on-review
rustbot
added
the
S-waiting-on-review
Amanieu
reviewed
Jul 25, 2025
| /// An `RwLock`, like [`Mutex`], will usually become poisoned on a panic. Note, | ||
| /// however, that an `RwLock` may only be poisoned if a panic occurs while it is | ||
| /// locked exclusively (write mode). If a panic occurs in any reader, then the | ||
| /// lock will not be poisoned. |
There was a problem hiding this comment.
This should point to the module-level documentation to explain the "usually".
There was a problem hiding this comment.
My PR describes the pitfalls in Mutex docs, rather than module-level docs, since they are specific to Mutex/RwLock and not other poisoning locks. I'll make "usually" point to the "Poisoning" section in Mutex docs instead, if that's okay with you.
| //! providing a way to deal with the poisoned state. | ||
| //! See [`Mutex`'s documentation](Mutex#poisoning) for more. | ||
| //! Panicking while holding the lock typically poisons the mutex, but it is | ||
| //! not guaranteed to detect this condition in all circumstances. |
There was a problem hiding this comment.
It would be good to clarify that poisoning is only unreliable when you have multiple pending panics or foreign exceptions. In particular it works fine in the "normal" case of a single panic.
There was a problem hiding this comment.
I wasn't sure if we wanted to commit to that guarantee. Consider the following situation:
- A panic is thrown.
- A drop guard locks a mutex and stores the guard into a cell. The guard records
panicking: true. - The drop guard exists and the panic is caught and dropped.
- Another panic is thrown.
- Another drop guard loads the guard from the cell and drops it. Since the guard has
panicking: trueand we're currently panicking as well, the mutex is not poisoned.
Yet there's never two active panics at the same time. Only one panic has been thrown while the mutex was locked -- the other panic was only caught.
That's a lot of detail I don't know if I can formalize in a few words in the docs without making a mistake and guaranteeing something we can't provide.
There was a problem hiding this comment.
The problem with your suggested docs is that the semantics are too vague to be of any use. Essentially we're saying that if a panic occurs while a MutexGuard is live then the lock may or may not be poisoned. While it's fine to not make hard guarantees, it would be nice to have a reassuring sentence along the lines of "if you're not doing fancy things like multiple live panics or preserving a MutexGuard across a catch_unwind then it will usually work as you expect".
There was a problem hiding this comment.
Or alternatively give specific examples of cases where poisoning doesn't work properly, with a note that this list isn't exhaustive. This will at least reassure people that poisoning probably works if they're not doing those things.
There was a problem hiding this comment.
Makes sense, I've added a list. Does this look good?
There was a problem hiding this comment.
Perfect! I'd just add another bullet point about foreign exceptions and this should be good to go.
There was a problem hiding this comment.
Totally forgot about that, thanks for the reminder. I added a note.
There was a problem hiding this comment.
@Amanieu sorry for pinging -- could I get an r+?
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
samueltardieu
added a commit
to samueltardieu/rust
that referenced
this pull request
Aug 2, 2025
…=Amanieu Document guarantees of poisoning This mostly documents the current behavior of `Mutex` and `RwLock` (rust-lang#143471) as imperfect. It's unlikely that the situation improves significantly in the future, and even if it does, the rules will probably be more complicated than "poisoning is completely reliable", so this is a conservative guarantee. We also explicitly specify that `OnceLock` never poisons, even though it has an API similar to mutexes. Fixes rust-lang#143471 by improving documentation. r? `@Amanieu`
bors
added a commit
that referenced
this pull request
Aug 2, 2025
Rollup of 18 pull requests Successful merges: - #132748 (get rid of some false negatives in rustdoc::broken_intra_doc_links) - #135771 ([rustdoc] Add support for associated items in "jump to def" feature) - #143360 (loop match: error on `#[const_continue]` outside `#[loop_match]`) - #143662 ([rustdoc] Display unsafe attrs with edition 2024 `unsafe()` wrappers.) - #143771 (Constify some more `Result` functions) - #143900 ([rustdoc] Correctly handle `should_panic` doctest attribute and fix `--no-run` test flag on the 2024 edition) - #144185 (Document guarantees of poisoning) - #144395 (update fortanix tests) - #144478 (Improve formatting of doc code blocks) - #144614 (Fortify RemoveUnneededDrops test.) - #144703 ([test][AIX] ignore extern_weak linkage test) - #144747 (compiletest: Improve diagnostics for line annotation mismatches 2) - #144756 (detect infinite recursion with tail calls in ctfe) - #144766 (Add human readable name "Cygwin") - #144782 (Properly pass path to staged `rustc` to `compiletest` self-tests) - #144786 (Cleanup the definition of `group_type`) - #144796 (Add my previous commit name to .mailmap) - #144797 (Update safety comment for new_unchecked in niche_types) Failed merges: - #144805 (compiletest: Preliminary cleanup of `ProcRes` printing/unwinding) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
that referenced
this pull request
Aug 2, 2025
Rollup of 17 pull requests Successful merges: - #132748 (get rid of some false negatives in rustdoc::broken_intra_doc_links) - #143360 (loop match: error on `#[const_continue]` outside `#[loop_match]`) - #143662 ([rustdoc] Display unsafe attrs with edition 2024 `unsafe()` wrappers.) - #143771 (Constify some more `Result` functions) - #144185 (Document guarantees of poisoning) - #144395 (update fortanix tests) - #144478 (Improve formatting of doc code blocks) - #144614 (Fortify RemoveUnneededDrops test.) - #144703 ([test][AIX] ignore extern_weak linkage test) - #144747 (compiletest: Improve diagnostics for line annotation mismatches 2) - #144756 (detect infinite recursion with tail calls in ctfe) - #144766 (Add human readable name "Cygwin") - #144782 (Properly pass path to staged `rustc` to `compiletest` self-tests) - #144786 (Cleanup the definition of `group_type`) - #144796 (Add my previous commit name to .mailmap) - #144797 (Update safety comment for new_unchecked in niche_types) - #144803 (rustc-dev-guide subtree update) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
that referenced
this pull request
Aug 3, 2025
Rollup merge of #144185 - purplesyringa:poisoning-wording, r=Amanieu Document guarantees of poisoning This mostly documents the current behavior of `Mutex` and `RwLock` (#143471) as imperfect. It's unlikely that the situation improves significantly in the future, and even if it does, the rules will probably be more complicated than "poisoning is completely reliable", so this is a conservative guarantee. We also explicitly specify that `OnceLock` never poisons, even though it has an API similar to mutexes. Fixes #143471 by improving documentation. r? ``@Amanieu``
github-actions bot
pushed a commit
to rust-lang/rustc-dev-guide
that referenced
this pull request
Aug 4, 2025
Rollup of 17 pull requests Successful merges: - rust-lang/rust#132748 (get rid of some false negatives in rustdoc::broken_intra_doc_links) - rust-lang/rust#143360 (loop match: error on `#[const_continue]` outside `#[loop_match]`) - rust-lang/rust#143662 ([rustdoc] Display unsafe attrs with edition 2024 `unsafe()` wrappers.) - rust-lang/rust#143771 (Constify some more `Result` functions) - rust-lang/rust#144185 (Document guarantees of poisoning) - rust-lang/rust#144395 (update fortanix tests) - rust-lang/rust#144478 (Improve formatting of doc code blocks) - rust-lang/rust#144614 (Fortify RemoveUnneededDrops test.) - rust-lang/rust#144703 ([test][AIX] ignore extern_weak linkage test) - rust-lang/rust#144747 (compiletest: Improve diagnostics for line annotation mismatches 2) - rust-lang/rust#144756 (detect infinite recursion with tail calls in ctfe) - rust-lang/rust#144766 (Add human readable name "Cygwin") - rust-lang/rust#144782 (Properly pass path to staged `rustc` to `compiletest` self-tests) - rust-lang/rust#144786 (Cleanup the definition of `group_type`) - rust-lang/rust#144796 (Add my previous commit name to .mailmap) - rust-lang/rust#144797 (Update safety comment for new_unchecked in niche_types) - rust-lang/rust#144803 (rustc-dev-guide subtree update) r? `@ghost` `@rustbot` modify labels: rollup
2 tasks
github-actions bot
pushed a commit
to model-checking/verify-rust-std
that referenced
this pull request
Aug 4, 2025
…=Amanieu Document guarantees of poisoning This mostly documents the current behavior of `Mutex` and `RwLock` (rust-lang#143471) as imperfect. It's unlikely that the situation improves significantly in the future, and even if it does, the rules will probably be more complicated than "poisoning is completely reliable", so this is a conservative guarantee. We also explicitly specify that `OnceLock` never poisons, even though it has an API similar to mutexes. Fixes rust-lang#143471 by improving documentation. r? ``@Amanieu``
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This mostly documents the current behavior of
MutexandRwLock(#143471) as imperfect. It's unlikely that the situation improves significantly in the future, and even if it does, the rules will probably be more complicated than "poisoning is completely reliable", so this is a conservative guarantee.We also explicitly specify that
OnceLocknever poisons, even though it has an API similar to mutexes.Fixes #143471 by improving documentation.
r? @Amanieu